-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update tests to provision 3 nodes cluster tests with a replicated loglet with replication property 2 #27
base: main
Are you sure you want to change the base?
Conversation
config/allowed-licenses.json
Outdated
{ | ||
"moduleLicense": "COMMON DEVELOPMENT AND DISTRIBUTION LICENSE (CDDL) Version 1.0" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This license is needed for javax.annotation:javax.annotation-api
which is pulled in by io.grpc:grpc-kotlin-stub
. It should not be a problem since we are not changing the sources.
@@ -46,24 +46,28 @@ class RestateContainer( | |||
private val WAIT_STARTUP_STRATEGY = | |||
WaitAllStrategy() | |||
.withStrategy( | |||
Wait.forHttp("/restate/health") | |||
.forPort(RUNTIME_INGRESS_ENDPOINT_PORT) | |||
Wait.forHttp("/metrics") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/metrics
will be reachable once the node grpc server starts up. Note at this point (if manual provisioning is enabled) the other services (ingress, admin, etc.) are not yet running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you put this as comment in code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do.
2aa6f7e
to
e4c415e
Compare
Gentle reminder @slinkydeveloper in case it went unnoticed before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what I can tell, makes sense to me. I left some small comments.
build.gradle.kts
Outdated
@@ -19,6 +21,8 @@ repositories { | |||
mavenCentral() | |||
// OSSRH Snapshots repo | |||
maven { url = uri("https://s01.oss.sonatype.org/content/repositories/snapshots/") } | |||
// for protobuf-gradle-plugin dependencies | |||
google() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?! aren't those on maven central? i never pulled from their maven repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me try to build w/o it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be not needed indeed. Will remove it.
@@ -46,24 +46,28 @@ class RestateContainer( | |||
private val WAIT_STARTUP_STRATEGY = | |||
WaitAllStrategy() | |||
.withStrategy( | |||
Wait.forHttp("/restate/health") | |||
.forPort(RUNTIME_INGRESS_ENDPOINT_PORT) | |||
Wait.forHttp("/metrics") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you put this as comment in code?
3a45dc5
to
4a94d8a
Compare
…let with replication property 2 This fixes restatedev#26.
4a94d8a
to
7edd9e8
Compare
"RESTATE_ALLOW_BOOTSTRAP" to "true", | ||
"RESTATE_BIFROST__DEFAULT_PROVIDER" to "replicated", | ||
"RESTATE_BIFROST__REPLICATED_LOGLET__DEFAULT_REPLICATION_PROPERTY" to "2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
This commit adds the NodeCtl grpc svc to the repo to generate a grpc client to manually provision a replicated loglet with a replciation property 2. W/o manually provisioning the cluster, the replication property defaults to 1.
Note: Whenever the *.proto files change in the restate repo, they need to be updated in this repository as well if there is an incompatible change.
This fixes #26.